-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make HtmlHelper::tag() just return the $text content with no wrapping tag when $name === false #1292
Conversation
… tag when $name === false
I must say I find that a little bit counter-intuitive. Especially since the documentation clearly states that it expects a string ( |
@@ -896,6 +896,9 @@ public function tableCells($data, $oddTrOptions = null, $evenTrOptions = null, $ | |||
* @link http://book.cakephp.org/2.0/en/core-libraries/helpers/html.html#HtmlHelper::tag | |||
*/ | |||
public function tag($name, $text = null, $options = array()) { | |||
if ($name === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use if (empty($name))
? You can't do anything meaningful if the name is a non-empty string anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, false/null/'' should all work the same ideally.
@dereuromark this was in reguards to the comments from @markstory on #1290. It's really about other helpers, e.g. PaginatorHelper, that use this. This allows for all of those to not force a wrapping tag. |
I see. Still dont like the idea of a non-stringish value to have some special treatment here - and think its better to use Admads approach using empy(). |
Shall I change it to (empty($name)) or just not do this at all? If we left it as $name === false we could throw an exception for when it's not a string. |
Thanks for taking the time to do this @openam :) |
👍 |
make HtmlHelper::tag() just return the $text content with no wrapping tag when $name === false
If
$name === false
then there shouldn't be a wrapping tag.